feat(github-app): check-run annotation builder - #608

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

reviewbot · advisory review

Reviewed 2 changed file(s) — two independent AI reviewers.

Suggested action: ✅ Safe to merge — the reviewer found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR adds a comprehensive check‑run annotation builder, extending advisory formatting to include inline annotations and handling limits. It introduces new types, constants, helper functions, and updates the output API, with extensive unit tests covering core scenarios.

Suggestions

• Consider simplifying annotationLineForFile – the current ternary always returns 1.
• Document the new annotationContext parameter in the function JSDoc for future maintainers.
• Verify that the imported CollisionCluster and CollisionReport types are exported from ../signals/engine.

Worth double-checking

• Existing callers must handle the optional annotations field; ensure no runtime assumptions break.
• The regex in isCodePath includes markdown files; confirm this aligns with intended hotspot detection.

Reviewer B · nemotron-3-120b-a12b — review unavailable

Superagent didn't find any vulnerabilities or security issues in this PR.

gittensory · advisory review

Reviewed 2 changed file(s) — two independent AI reviewers.

Suggested action: ✅ Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR adds a comprehensive check‑run annotation builder, extending advisory output with structured annotations and thorough unit tests. The implementation is clean, respects the existing API, and the new functionality is well‑covered by tests.

Suggestions

• Consider adding a comment clarifying that annotationLineForFile currently always returns line 1, in case future logic is needed.
• Document the new optional annotationContext parameter in the function’s JSDoc to aid future developers.
• Verify that any barrel (index.ts) files re‑export the new types/constants if they are used outside this module.

Worth double-checking

• Existing callers of formatCheckRunOutput now receive an extended return type; ensure no code relies on exact shape without allowing extra fields.
• The added CollisionCluster import is only used for typing; ensure the type is exported from ../signals/engine to avoid TypeScript errors.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
This PR adds GitHub Check Run annotation generation to advisory results, enabling inline annotations for missing tests, duplicate overlaps, and public findings. The implementation is thorough with comprehensive unit tests covering edge cases, deduplication, annotation limits, and detail levels. Changes are localized to advisory.ts and its test file, maintaining backward compatibility.

Suggestions

• Consider improving annotationLineForFile to reference a more meaningful line (e.g., first changed line) rather than always returning line 1 for better annotation usefulness.
• Evaluate whether collision annotations should be limited to files actually involved in the overlap (if such data is available) to reduce noise, though current PR-level approach is acceptable.

Worth double-checking

• The annotationLineForFile function always returns line 1, which may reduce annotation usefulness if not improved in future work.
• Ensure that the added import types (PullRequestFileRecord, CollisionReport) exist and are correctly referenced to avoid build/runtime errors.
• Verify that the sanitizeForCheckRun function adequately handles GitHub's annotation limits (though tests cover length truncation).